frontend/javascripts/oxalis/model/sagas/volume/floodfill_saga.tsx (1)

35-102: Ensure proper handling of failure reasons in getBoundingBoxForFloodFill

In the getBoundingBoxForFloodFill function, when returning failure reasons, ensure that the failure messages are user-friendly and informative. This will enhance the user's understanding when the flood fill operation cannot proceed.

frontend/javascripts/test/sagas/volumetracing/volumetracing_saga_integration.spec.ts (1)

Line range hint 330-347: Improve test reliability with explicit undo actions

When performing undo and redo operations in the test, ensure that each action is awaited properly to prevent race conditions and enhance test stability.

Apply this diff to await undo and redo actions explicitly:

- await dispatchUndoAsync(Store.dispatch);
+ await dispatchUndoAsync(Store.dispatch);
+ await t.context.api.tracing.save();

- await dispatchRedoAsync(Store.dispatch);
+ await dispatchRedoAsync(Store.dispatch);
+ await t.context.api.tracing.save();

frontend/stylesheets/trace_view/_action_bar.less (1)

71-74: Consider using a more semantic class name

The class name .undo-redo-button suggests specific usage, but the functionality (preventing width flickering) could be useful for other buttons. Consider a more generic name like .fixed-width-button or .no-flicker-button.

CHANGELOG.unreleased.md (1)

16-16: Enhance the changelog entry with more details

Consider expanding the changelog entry to provide more comprehensive information about the feature:

-The fill tool can now be adapted so that it only acts within a specified bounding box. Use the new "Restrict Floodfill" mode for that in the toolbar. [#8267](https://github.com/scalableminds/webknossos/pull/8267)
+The fill tool can now be restricted to operate within a specified bounding box. Enable the new "Restrict Floodfill" toggle in the toolbar when the fill tool is selected. If no bounding box exists when using this mode, a notification will be displayed. This feature helps prevent unintended fill operations outside the area of interest. [#8267](https://github.com/scalableminds/webknossos/pull/8267)

frontend/javascripts/oxalis/default_state.ts (1)

87-87: LGTM! Consider adding JSDoc

The new configuration property is well-named and appropriately placed. Consider adding JSDoc documentation to describe its purpose and behavior:

+    /** When enabled, restricts flood fill operations to the current bounding box */
     isFloodfillRestrictedToBoundingBox: false,

frontend/javascripts/oxalis/constants.ts (1)

336-338: Consider adding more detailed documentation.

While the comment explains the purpose, it would be helpful to document:

1. Why the value of 10 was chosen
2. What "more lax" means in terms of actual behavior
3. The impact on performance and memory usage

frontend/javascripts/oxalis/model/bucket_data_handling/data_cube.ts (2)

501-509: Consider adding more detailed documentation for the dynamic bounding box expansion

While the implementation is solid, the comment could be more explicit about:

• The purpose of allowing multiple additions of the same bucket
• The implications of the "neighbour volume shape" concept
• How this relates to the new bounding box restriction feature

---

695-724: Consider enhancing the progress logging

The progress logging implementation could be improved:

1. Consider using a constant for the progress update interval (1000000)
2. Consider adding an estimate of total progress (e.g., percentage complete)
3. Consider adding more context about the operation in the progress message

-                if (labeledVoxelCount % 1000000 === 0) {
-                  console.log(`Labeled ${labeledVoxelCount} Vx. Continuing...`);
-
-                  await progressCallback(
-                    false,
-                    `Labeled ${labeledVoxelCount / 1000000} MVx. Continuing...`,
-                  );
+                const PROGRESS_UPDATE_INTERVAL = 1000000;
+                if (labeledVoxelCount % PROGRESS_UPDATE_INTERVAL === 0) {
+                  const message = `Flood fill: Labeled ${(labeledVoxelCount / 1000000).toFixed(1)} MVx`;
+                  console.log(message);
+                  await progressCallback(false, message);

frontend/javascripts/oxalis/view/action-bar/toolbar_view.tsx (1)

1372-1405: Consider enhancing the FloodFillSettings component

While the implementation is solid, consider these improvements:

1. Extract the tooltip text to a constant for reusability
2. Add a data-testid attribute for easier testing
3. Consider using a memo hook for the toggle callback to prevent unnecessary re-renders

+const FLOOD_FILL_BBOX_TOOLTIP = 
+  "When enabled, the floodfill will be restricted to the bounding box enclosed by the clicked position. If multiple bounding boxes enclose that position, the smallest is used.";

 function FloodFillSettings() {
   const dispatch = useDispatch();
   const isRestrictedToBoundingBox = useSelector(
     (state: OxalisState) => state.userConfiguration.isFloodfillRestrictedToBoundingBox,
   );
-  const toggleRestrictFloodfillToBoundingBox = () => {
+  const toggleRestrictFloodfillToBoundingBox = useCallback(() => {
     dispatch(
       updateUserSettingAction("isFloodfillRestrictedToBoundingBox", !isRestrictedToBoundingBox),
     );
-  };
+  }, [dispatch, isRestrictedToBoundingBox]);

   return (
-    <div>
+    <div data-testid="flood-fill-settings">
       <FillModeSwitch />

       <ButtonComponent
         style={{
           opacity: isRestrictedToBoundingBox ? 1 : 0.5,
           marginLeft: 12,
         }}
         type={isRestrictedToBoundingBox ? "primary" : "default"}
         onClick={toggleRestrictFloodfillToBoundingBox}
-        title={
-          "When enabled, the floodfill will be restricted to the bounding box enclosed by the clicked position. If multiple bounding boxes enclose that position, the smallest is used."
-        }
+        title={FLOOD_FILL_BBOX_TOOLTIP}
       >

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: dieknolle3333
PR: scalableminds/webknossos#8168
File: frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx:433-434
Timestamp: 2024-11-22T17:19:07.947Z
Learning: In the codebase, certain usages of `segmentationLayer.resolutions` are intentionally retained and should not be changed to `segmentationLayer.mags` during refactoring.

 export const getUserBoundingBoxesThatContainPosition = (
   state: OxalisState,
   position: Vector3,
 ): Array<UserBoundingBox> => {
   const bboxes = getUserBoundingBoxesFromState(state);
+  const boundingBoxInstances = bboxes.map(
+    (el) => new BoundingBox(el.boundingBox)
+  );
 
-  return bboxes.filter((el) => new BoundingBox(el.boundingBox).containsPoint(position));
+  return bboxes.filter((_, idx) => 
+    boundingBoxInstances[idx].containsPoint(position)
+  );
 };

docs/volume_annotation/tools.md (1)

71-72: Enhance the documentation with additional implementation details.

The documentation provides a good overview of the new feature, but could be enhanced with the following details from the PR objectives:

1. Clarify the toggle behavior:
   • When disabled: Flood fill operates normally
   • When enabled: Fill is limited to the bounding box
2. Add a note about the notification shown when no bounding box exists
3. Explain the 3D mode behavior regarding early termination and bounding box creation

Consider expanding the section with this additional content:

 ![Restrict Fill](./images/icon_restricted_floodfill.jpg){align=left width="60"} 
 **Restrict Fill by Bounding Box**: When enabled, the fill operation will be restricted by the smallest bounding box that encloses the clicked position. This feature can be useful when correcting segmentation in a small bounding box (e.g., when curating training data).
+
+When the toggle is disabled, flood fill operates normally. When enabled, the fill will be limited to the bounding box encompassing the clicked position. If no bounding box exists, you'll receive a notification. In 3D mode, a new bounding box will only be created on early termination if the restriction is disabled.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

frontend/javascripts/oxalis/model/sagas/volume/floodfill_saga.tsx (2)

53-54: Consider extracting repeated volume calculation logic into a helper function.

Here, you use new BoundingBox(bbox.boundingBox).getVolume() while also using a similar logic for 2D calculations below. Factoring out this logic would reduce duplication and improve readability.

---

69-72: Provide localized or more concise error messaging if needed.

The error message might be displayed to end-users verbatim. If you plan to support multiple languages or wish to keep messages more concise, consider externalizing or simplifying the string.

frontend/javascripts/libs/progress_callback.ts (1)

14-14: Document the final feedback method parameter.

The “finalFeedbackMethod” is intuitive, but consider adding a brief line in the code comments about acceptable values (“success”, “error”, “info”, “warning”) for self-documentation.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

frontend/javascripts/test/sagas/saga_integration.mock.ts (1)

18-21: LGTM! Consider clarifying the comment.

The changes improve the mock's fidelity by returning hide functions, which better matches the actual antd message API behavior.

Consider making the comment more explicit about the API behavior:

-    // These return a "hide function"
+    // These methods return a function that can be called to hide/close the message

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro